Skip to content

[APPS] Extract inline backend connection IDs#346

Draft
sdkennedy2 wants to merge 1 commit intosdkennedy2/add-ast-scope-walkerfrom
sdkennedy2/inline-connection-id-strings
Draft

[APPS] Extract inline backend connection IDs#346
sdkennedy2 wants to merge 1 commit intosdkennedy2/add-ast-scope-walkerfrom
sdkennedy2/inline-connection-id-strings

Conversation

@sdkennedy2
Copy link
Copy Markdown
Collaborator

@sdkennedy2 sdkennedy2 commented May 7, 2026

Motivation

#338 added root manifest.json upload plumbing and required allowedConnectionIds arrays for backend functions, but those arrays are still empty. This PR is the first behavior-changing PR in the new discovery parsing stack: it fills allowlists for the safest static case, inline string literal connectionId values inside the backend entry module.

Changes

Adds a new AST extractor under backend/ast-parsing that scans the already-parsed backend file for action-catalog imports and direct action-catalog calls. It recognizes @datadog/action-catalog package root and subpaths, supports named/default/namespace imports, ignores type-only imports, and ignores unrelated functions that happen to accept a connectionId property.

The extractor intentionally stays file-local for this PR. It accepts only inline string literals like:

import { request } from '@datadog/action-catalog/http/http';

request({ connectionId: '77c14b8b-27e1-4901-985d-8817908b9706', inputs: {} });

Unsupported action-catalog shapes that could hide a connection ID fail closed, including identifier/template/member/call expressions, object spreads, computed keys, non-object call arguments, and optional action calls. Extracted IDs are deduped, sorted, and assigned as the same file-level union to every backend export from that entry file.

QA Instructions

Run:

~/.yarn/switch/bin/yarn eslint packages/plugins/apps/src/backend/ast-parsing/extract-connection-ids.ts packages/plugins/apps/src/backend/ast-parsing/extract-connection-ids.test.ts packages/plugins/apps/src/index.ts packages/plugins/apps/src/index.test.ts --quiet
~/.yarn/switch/bin/yarn workspace @dd/apps-plugin typecheck
~/.yarn/switch/bin/yarn workspace @dd/tests test:unit packages/plugins/apps/src/backend/ast-parsing/extract-connection-ids.test.ts packages/plugins/apps/src/index.test.ts --runInBand
~/.yarn/switch/bin/yarn workspace @dd/tests test:unit packages/plugins/apps/src/backend/discovery.test.ts packages/plugins/apps/src/vite/dev-server.test.ts --runInBand

I also verified against /Users/scott.kennedy/dd/test-action-catalog-app by capturing the uploaded ZIP from npm run build and inspecting manifest.json. The helper path backend/helpers/httpProbe.ts uses CONNECTIONS.HTTP through an imported helper module, so this PR correctly leaves moduleGraphHttpProbe.allowedConnectionIds empty; later stack PRs own same-module and module-graph resolution.

Blast Radius

This affects only @dd/apps-plugin backend discovery for High Code Apps. The frontend proxy shape is unchanged. The manifest allowlist remains empty unless a backend entry file contains supported inline action-catalog connectionId strings. Known action-catalog calls with unsupported connection ID shapes now fail during build instead of silently emitting an incomplete allowlist.

Documentation

  • No docs change for this internal stack PR.

Copy link
Copy Markdown
Collaborator Author

sdkennedy2 commented May 7, 2026

Warning

This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more

This stack of pull requests is managed by Graphite. Learn more about stacking.

@sdkennedy2 sdkennedy2 changed the title Extract inline backend connection IDs [APPS] Extract inline backend connection IDs May 7, 2026
@sdkennedy2
Copy link
Copy Markdown
Collaborator Author

@codex review
@cursor review

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 15470da044

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +195 to +196
if (property.kind !== 'init') {
continue;
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Reject accessor connectionId properties

When an action-catalog call uses an accessor like request({ get connectionId() { return CONNECTIONS.HTTP; } }), ESTree reports this as a Property whose kind is get; this branch skips it and the extractor returns no ID, so the manifest can be emitted with an empty allowlist even though runtime reads a connection ID. Since non-literal connection IDs are meant to fail closed, accessor properties named connectionId should throw instead of being ignored.

Useful? React with 👍 / 👎.

Comment on lines +292 to +293
default:
walkChildNodes(node, scope, importedNames, visit);
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Track loop and catch shadowing bindings

The generic traversal visits ForOfStatement/ForInStatement/CatchClause children with the unchanged outer scope, so bindings introduced by those constructs don't shadow action-catalog imports. In a backend such as for (const request of handlers) request({ connectionId: id }), that local callback is classified as the imported action-catalog request, which can either fail the build on a dynamic value or pollute the allowlist for a non-action call; please add scoped handling for these binding sites before walking their bodies.

Useful? React with 👍 / 👎.

@sdkennedy2 sdkennedy2 force-pushed the sdkennedy2/inline-connection-id-strings branch from 15470da to aa6575e Compare May 7, 2026 17:58
@sdkennedy2 sdkennedy2 force-pushed the sdkennedy2/refactor-discovery-ast-parsing branch from 8a9e82f to e869c8d Compare May 7, 2026 17:58
Base automatically changed from sdkennedy2/refactor-discovery-ast-parsing to master May 7, 2026 18:11
@sdkennedy2 sdkennedy2 changed the base branch from master to graphite-base/346 May 7, 2026 18:40
@sdkennedy2 sdkennedy2 force-pushed the sdkennedy2/inline-connection-id-strings branch from aa6575e to 902027b Compare May 7, 2026 18:40
@sdkennedy2 sdkennedy2 changed the base branch from graphite-base/346 to sdkennedy2/add-ast-scope-walker May 7, 2026 18:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant